feat(inbound filters): Enable filtering sessions #4745
Conversation
| &self.inner.global_config.current(), | ||
| &config, |
There was a problem hiding this comment.
Here, we are getting passed the config from outside, but reading the global_config from self.inner. I did it this way because that's how it is in other processors (e.g. replays), but I don't know whether there's a good reason for it or it's a historical accident.
loewenheim
changed the title
loewenheim
force-pushed
the
sebastian/filter-sessions
branch
from
b97cba9 to
07b028b
Compare
|
I've now reduced the |
That's the question, do you need even any fields filterable (aka a Getter impl that can get anything)?
Usually the JSON path of the fields, since sessions are more like metrics, this may not make sense. |
loewenheim
force-pushed
the
sebastian/filter-sessions
branch
from
c1201d1 to
3ca5562
Compare
tests/integration/test_session.py
Outdated
| "sid": "8333339f-5675-4f89-a9a0-1c935255ab59", | ||
| "timestamp": timestamp.isoformat(), | ||
| "started": timestamp.isoformat(), | ||
| "attrs": {"release": "[email protected]", "ip_address": "1.2.3.0/24"}, |
There was a problem hiding this comment.
Does this IP filtering match the one we have for errors? I think there was something like, that we do not filter on user ips with that denylist, but actual ingestion IPs. We should follow the same here, if it filters on the user ip, this would be a different behaviour.
There was a problem hiding this comment.
The documentation of Filterable::ip_addr describes it as returning the client IP:
/// The IP address of the client that sent the data.
fn ip_addr(&self) -> Option<&str>;
And the impl for Event also seems to use the client IP:
fn ip_addr(&self) -> Option<&str> {
let user = self.user.value()?;
Some(user.ip_address.value()?.as_ref())
}
|
@Dav1dde I added a paragraph about client and user IPs to the |
Filtering of sessions was implemented in getsentry/relay#4745. I'm leaving the comment about minidumps in place because I'm honestly not sure what it's supposed to mean.
This enables filtering for
SessionUpdateandSessionAggregateitems. In the course of this, it alsoFilterableandGetterimplementations toSessionUpdateandSessionAggregates;SessionProcessingConfigstruct to bundle all the auxiliary data needed inprocess_sessionandprocess_session_aggregates(modeled onReplayProcessingConfig);Aside from this not having tests, my main open questions are around the
Getterimpls. To wit:"session_update"/"session_aggregates")?Getterimpls change anything about the way filtering works? I believeFilterablealready returns the fields we actually care about.Closes RELAY-41.